Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

t/ckeditor5-media-embed/35: Implemented LabeledInputView#infoText to display useful hints next to the input #444

Merged
merged 6 commits into from
Oct 29, 2018

Conversation

oleq
Copy link
Member

@oleq oleq commented Sep 26, 2018

Suggested merge commit message (convention)

Feature: Implemented LabeledInputView#infoText to display useful hints next to the input (see ckeditor/ckeditor5#2753).


Additional information

A piece of ckeditor/ckeditor5-media-embed#45.

@coveralls
Copy link

coveralls commented Sep 26, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling e2bd47c on t/ckeditor5-media-embed/35 into 85920b5 on master.

@Reinmar
Copy link
Member

Reinmar commented Sep 26, 2018

Are we sure we want to extend this component? Shouldn't that additional info text be displayed by the feature itself? What if we'd want an info text next to some other type of input field? And is it really useful that this text is displayed by this component? Can it be done differently?

@oleq
Copy link
Member Author

oleq commented Oct 5, 2018

I was thinking about that and I guess we have 3 choices:

  • stick to the implementation in this PR,

  • implement it on the MediaFormView level, which isn't very pretty because it means we have to wrap the input in some additional container, then render the info text there, which could be cumbersome because

    • we'd have to take care about not displaying it when the field has an error (because it's way too much),
    • we'd have to maintain its styles separately so they correspond to the input error text (font, spacing); if we changed anything in the styles of the UI library, we'd need to get back to the media embed styles (+ whatever features implement info text) and update them, which is quite a hassle IMO
  • implement a form manager; I'm not sure if we have an issue for that but the more forms we have, the more useful a utility to generate them would become. Such a util could take atomic components like inputs, buttons and put them together with

    • validation,
    • additional info texts,
    • whatever magic advanced forms require

    but it feels like a lot of design work and coding especially that it's not like we need it so badly ATM.

@Reinmar
Copy link
Member

Reinmar commented Oct 26, 2018

OK, sounds reasonable.

@oskarwrobel
Copy link
Contributor

I'll look at this on Monday.

@oleq oleq merged commit 6ac03ea into master Oct 29, 2018
@oleq oleq deleted the t/ckeditor5-media-embed/35 branch October 29, 2018 16:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants